Skip to content

Conversation

@SchrodingerZhu
Copy link
Contributor

  • migrate more -O3 to ${libc_opt_high_flag}
  • workaround a issue with LLP64 in test. The overflow testing is guarded by a constexpr but the literal overflow itself will still trigger warnings.

Notice that for math smoke test, for some reasons, the ${libc_opt_high_flag} will be passed into lld-link which confuses the linker so there are still some warnings leftover there. I can investigate more when I have time.

@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2024

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes
  • migrate more -O3 to ${libc_opt_high_flag}
  • workaround a issue with LLP64 in test. The overflow testing is guarded by a constexpr but the literal overflow itself will still trigger warnings.

Notice that for math smoke test, for some reasons, the ${libc_opt_high_flag} will be passed into lld-link which confuses the linker so there are still some warnings leftover there. I can investigate more when I have time.


Full diff: https://github.com/llvm/llvm-project/pull/117718.diff

10 Files Affected:

  • (modified) libc/src/__support/threads/linux/CMakeLists.txt (+1-1)
  • (modified) libc/src/complex/generic/CMakeLists.txt (+10-10)
  • (modified) libc/src/pthread/CMakeLists.txt (+1-1)
  • (modified) libc/src/setjmp/riscv/CMakeLists.txt (+2-2)
  • (modified) libc/src/setjmp/x86_64/CMakeLists.txt (+2-2)
  • (modified) libc/src/signal/linux/CMakeLists.txt (+1-1)
  • (modified) libc/src/stdfix/CMakeLists.txt (+7-7)
  • (modified) libc/src/string/CMakeLists.txt (+1-1)
  • (modified) libc/src/threads/CMakeLists.txt (+1-1)
  • (modified) libc/test/src/math/smoke/LdExpTest.h (+2-1)
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index fa11458f99b6c9..47598d98c98863 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -88,7 +88,7 @@ add_object_library(
     libc.src.__support.error_or
     libc.src.__support.threads.thread_common
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
     -fno-omit-frame-pointer # This allows us to sniff out the thread args from
                             # the new thread's stack reliably.
     -Wno-frame-address      # Yes, calling __builtin_return_address with a
diff --git a/libc/src/complex/generic/CMakeLists.txt b/libc/src/complex/generic/CMakeLists.txt
index a63871b32afbf1..a3da781c602378 100644
--- a/libc/src/complex/generic/CMakeLists.txt
+++ b/libc/src/complex/generic/CMakeLists.txt
@@ -5,7 +5,7 @@ add_entrypoint_object(
   HDRS
     ../creal.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -18,7 +18,7 @@ add_entrypoint_object(
   HDRS
     ../crealf.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -31,7 +31,7 @@ add_entrypoint_object(
   HDRS
     ../creall.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -44,7 +44,7 @@ add_entrypoint_object(
   HDRS
     ../crealf16.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -59,7 +59,7 @@ add_entrypoint_object(
   HDRS
     ../crealf128.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -74,7 +74,7 @@ add_entrypoint_object(
   HDRS
     ../cimag.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -87,7 +87,7 @@ add_entrypoint_object(
   HDRS
     ../cimagf.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -100,7 +100,7 @@ add_entrypoint_object(
   HDRS
     ../cimagl.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -113,7 +113,7 @@ add_entrypoint_object(
   HDRS
     ../cimagf16.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
@@ -128,7 +128,7 @@ add_entrypoint_object(
   HDRS
     ../cimagf128.h
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.CPP.bit
     libc.src.__support.complex_type
diff --git a/libc/src/pthread/CMakeLists.txt b/libc/src/pthread/CMakeLists.txt
index 8480fd89422223..c8c66805667fa9 100644
--- a/libc/src/pthread/CMakeLists.txt
+++ b/libc/src/pthread/CMakeLists.txt
@@ -333,7 +333,7 @@ add_entrypoint_object(
     libc.src.pthread.pthread_attr_getstack
     libc.src.errno.errno
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
     -fno-omit-frame-pointer
 )
 
diff --git a/libc/src/setjmp/riscv/CMakeLists.txt b/libc/src/setjmp/riscv/CMakeLists.txt
index 464144758cbbea..c68e318a0f08ba 100644
--- a/libc/src/setjmp/riscv/CMakeLists.txt
+++ b/libc/src/setjmp/riscv/CMakeLists.txt
@@ -7,7 +7,7 @@ add_entrypoint_object(
   DEPENDS
     libc.hdr.types.jmp_buf
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
     -fomit-frame-pointer
 )
 
@@ -20,6 +20,6 @@ add_entrypoint_object(
   DEPENDS
     libc.hdr.types.jmp_buf
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
     -fomit-frame-pointer
 )
diff --git a/libc/src/setjmp/x86_64/CMakeLists.txt b/libc/src/setjmp/x86_64/CMakeLists.txt
index b5b0d9ba65599c..96d5751bc81dd2 100644
--- a/libc/src/setjmp/x86_64/CMakeLists.txt
+++ b/libc/src/setjmp/x86_64/CMakeLists.txt
@@ -7,7 +7,7 @@ add_entrypoint_object(
   DEPENDS
     libc.hdr.types.jmp_buf
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
 )
 
 add_entrypoint_object(
@@ -19,6 +19,6 @@ add_entrypoint_object(
   DEPENDS
     libc.hdr.types.jmp_buf
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
     -fomit-frame-pointer
 )
diff --git a/libc/src/signal/linux/CMakeLists.txt b/libc/src/signal/linux/CMakeLists.txt
index e314242dc376fb..f7457d31cf4f85 100644
--- a/libc/src/signal/linux/CMakeLists.txt
+++ b/libc/src/signal/linux/CMakeLists.txt
@@ -41,7 +41,7 @@ add_object_library(
     __restore.cpp
   COMPILE_OPTIONS
     -fomit-frame-pointer
-    -O3
+    ${libc_opt_high_flag}
     -Wframe-larger-than=0
     -Wno-attributes
     # asan creates asan.module_ctor which uses stack space, causing warnings.
diff --git a/libc/src/stdfix/CMakeLists.txt b/libc/src/stdfix/CMakeLists.txt
index 238b86728188ad..815f739d23efa4 100644
--- a/libc/src/stdfix/CMakeLists.txt
+++ b/libc/src/stdfix/CMakeLists.txt
@@ -10,7 +10,7 @@ foreach(suffix IN ITEMS hr r lr hk k lk)
     SRCS
       abs${suffix}.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.__support.fixed_point.fx_bits
   )
@@ -24,7 +24,7 @@ foreach(suffix IN ITEMS uhr ur ulr uhk uk)
     SRCS
       sqrt${suffix}.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.__support.fixed_point.sqrt
   )
@@ -38,7 +38,7 @@ foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
     SRCS
       round${suffix}.cpp
     COMPILE_OPTIONS
-      -O3
+      ${libc_opt_high_flag}
     DEPENDS
       libc.src.__support.fixed_point.fx_bits
   )
@@ -62,7 +62,7 @@ add_entrypoint_object(
   SRCS
     uhksqrtus.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.fixed_point.sqrt
 )
@@ -74,7 +74,7 @@ add_entrypoint_object(
   SRCS
     uksqrtui.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.fixed_point.sqrt
 )
@@ -86,7 +86,7 @@ add_entrypoint_object(
   SRCS
     exphk.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.fixed_point.fx_rep
     libc.src.__support.CPP.bit
@@ -99,7 +99,7 @@ add_entrypoint_object(
   SRCS
     expk.cpp
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
   DEPENDS
     libc.src.__support.fixed_point.fx_rep
     libc.src.__support.CPP.bit
diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index d6e300754d4f9c..8fe1226dd6a7a5 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -506,7 +506,7 @@ function(add_implementation name impl_name)
     SRCS ${ADD_IMPL_SRCS}
     HDRS ${ADD_IMPL_HDRS}
     DEPENDS ${ADD_IMPL_DEPENDS}
-    COMPILE_OPTIONS -O3 ${ADD_IMPL_COMPILE_OPTIONS}
+    COMPILE_OPTIONS ${libc_opt_high_flag} ${ADD_IMPL_COMPILE_OPTIONS}
   )
   get_fq_target_name(${impl_name} fq_target_name)
   set_target_properties(${fq_target_name} PROPERTIES REQUIRE_CPU_FEATURES "${ADD_IMPL_REQUIRE}")
diff --git a/libc/src/threads/CMakeLists.txt b/libc/src/threads/CMakeLists.txt
index c33be265369fec..17dea3920a07aa 100644
--- a/libc/src/threads/CMakeLists.txt
+++ b/libc/src/threads/CMakeLists.txt
@@ -24,7 +24,7 @@ add_entrypoint_object(
     libc.include.threads
     libc.src.errno.errno
   COMPILE_OPTIONS
-    -O3
+    ${libc_opt_high_flag}
     -fno-omit-frame-pointer # This allows us to sniff out the thread args from
                             # the new thread's stack reliably.
 )
diff --git a/libc/test/src/math/smoke/LdExpTest.h b/libc/test/src/math/smoke/LdExpTest.h
index 42e4c6a3a5184f..094cc7e4e9e7d0 100644
--- a/libc/test/src/math/smoke/LdExpTest.h
+++ b/libc/test/src/math/smoke/LdExpTest.h
@@ -50,7 +50,8 @@ class LdExpTestTemplate : public LIBC_NAMESPACE::testing::FEnvSafeTest {
 
     if constexpr (sizeof(U) < sizeof(long) || sizeof(long) == sizeof(int))
       return;
-    long long_exp_array[4] = {LONG_MIN, INT_MIN - 1L, INT_MAX + 1L, LONG_MAX};
+    long long_exp_array[4] = {LONG_MIN, static_cast<long>(INT_MIN - 1LL),
+                              static_cast<long>(INT_MAX + 1LL), LONG_MAX};
     for (long exp : long_exp_array) {
       ASSERT_FP_EQ(zero, func(zero, exp));
       ASSERT_FP_EQ(neg_zero, func(neg_zero, exp));

@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Nov 26, 2024
@SchrodingerZhu SchrodingerZhu merged commit c714185 into llvm:main Nov 26, 2024
11 of 12 checks passed
@nickdesaulniers
Copy link
Member

workaround a issue with LLP64 in test. The overflow testing is guarded by a constexpr but the literal overflow itself will still trigger warnings.

Would a preprocessor guard be more appropriate then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel libc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants